fix(Spanner): Make System Tests Work & Work Faster by cy-yun · Pull Request #9401 · googleapis/google-cloud-php

cy-yun · 2026-07-27T21:01:29Z

This PR addresses the ongoing flakiness, high database provisioning overhead, and parallel-execution collisions of the Spanner system tests.

By consolidating test database provisioning, we reduce overall execution time through database reuse. Furthermore, we've removed structural bottlenecks (such as ID collisions and schema mismatches) that prevented these tests from running concurrently. (Parallelizing these tests with paratest to bring the time down to minutes locally is planned for a subsequent follow-up).

Below is a detailed breakdown of the specific problems and fixes implemented in this PR:

1. Test Database Provisioning Overhead & DDL Consolidation

Problem: The system test suite was creating and dropping a new test database for every single test class, adding massive overhead to the already long execution time. Furthermore, individual test classes independently executed DDL statements (CREATE TABLE), causing Table already exists collisions when tests were executed against a shared persistent database.
Fix:
- Introduced TestDatabaseManager to provision a single shared Spanner test database and reuse it across all test classes.
- Centralized all table and schema creation logic into unified traits: SystemTestCaseTrait.php (Standard Spanner) and PgSystemTestCaseTrait.php (Postgres Spanner).
- Removed redundant CREATE TABLE and DROP TABLE operations from over a dozen individual test classes (e.g., BatchTest, WriteTest, PgBatchTest).

2. PostgreSQL DDL Compatibility & Schema Mismatches

Problem: Reusing the same database across multiple test runs or classes caused PostgreSQL CREATE TABLE commands to fail because the tables already existed. Additionally, there was schema drift between individual tests.
Fix:
- Added IF NOT EXISTS to PostgreSQL DDL statements in the Pg*Test classes.
- Reconciled schema drift and mismatches in PgQueryTest and PgPartitionedDmlTest to align exactly with the unified setup suite definitions.
- Adjusted PgReadTest.php so that secondary index creation operates safely on shared databases without conflicting with concurrent tests.

3. Row ID Collisions on Persistent Databases

Problem: Because multiple tests (like TransactionTest, OperationsTest, BackupTest) share the same Users test table, tests running sequentially on a persistent database were slowly filling up the table. Tests generated random keys using narrow ranges like rand(1, 346464), leading to frequent ALREADY_EXISTS errors during insertion due to the Birthday Paradox.
Fix:
- Updated the central SystemTestCase::randId() generator to use a massive 1-billion key space: rand(1, 999999999).
- Replaced 11 instances of hardcoded, narrow rand() ranges in TransactionTest.php and PgTransactionTest.php to use the unified self::randId() helper.

4. `BackupTest` Reliability & Timeouts

Problem: BackupTest operations were flaking due to transient errors, DEADLINE_EXCEEDED errors when GCP was slow, and improper polling configurations.
Fix:
- Updated long-running operation polling to use maxPollingDurationSeconds via an extended timeout loop for all Backup and Restore operations.
- Refactored the timeout try/catch logic into a DRY pollWithExtendedTimeout($op) helper method in BackupTest.
- Caught and ignored DEADLINE_EXCEEDED exceptions emitted during intermediate polling attempts.

5. Emulator Transaction Leaks

Problem: When running tests against the emulator, unhandled or failed transactions were leaking and persisting on the emulator, causing subsequent test failures or slowdowns.
Fix: Added an unhandled exception catch block in Database::runTransaction to rollback non-single-use active transactions before bubbling up the exception. Ensured ReadTest.php properly cleans up local emulator transaction state.

6. CI Configuration

Problem: Spanner system tests were not executing in the standard CI pipeline.
Fix: Enabled the Spanner test suite in phpunit-system.xml.dist now that stability is restored.

test(spanner): consolidate test database provisioning fix(spanner): fix PgReadTest index creation on shared database test(spanner): Consolidate DDL operations into test setup suite test(spanner): fix schema mismatches for partitionedDml and PgQueryTest test(spanner): rename PgQueryTest_2 back to PgQueryTest test(spanner): fix PostgreSQL column name mismatches for test tables

test(spanner): Handle DEADLINE_EXCEEDED in BackupTest test(spanner): fix BackupTest timeout and PgQueryTest schema collision test(spanner): add deadline exceeded polling to testCreateBackup2 refactor(spanner): DRY up extended polling loop in BackupTest

…ger range test(spanner): restore seedTable in BatchTest to fix test coverage test(spanner): fix array_rand bug and batch mutations in seedTable test(spanner): fix fundamentally broken partitionRead test logic in BatchTest fix: move seedTable back to its original location fix(cs): fix style issues in BatchTest.php fix(cs): format multi-line args

…tent databases, and apply code review fixes for tests

Hectorhammett · 2026-07-30T23:35:17Z

+        for ($i = 0; $i < 3; $i++) {
+            try {
+                $backup->create(self::$dbName1, $expireTime);
+                break;
+            } catch (BadRequestException $e) {
+                break;
+            } catch (FailedPreconditionException $e) {
+                break;
+            } catch (\Google\Cloud\Core\Exception\ServiceException $ex) {
+                $allowed = [14 /* UNAVAILABLE */, 4 /* DEADLINE_EXCEEDED */];
+                if ($i === 2 || !in_array($ex->getCode(), $allowed)) {
+                    throw $ex;
+                }
+                sleep(2);
+            }


I am having a hard time understanding this. Why are we looping up to 3, but we just break after the first retry?

It seems like we want to catch an exception for $e and then assert that is not null.

What do we need the loop for?

Hectorhammett · 2026-07-30T23:36:38Z

+        try {
+            $op->cancel();
+        } catch (\Google\Cloud\Core\Exception\ServiceException $e) {
+            if ($e->getCode() !== 4 /* DEADLINE_EXCEEDED */) {


Not a big fan of "magic numbers"

I believe there is a Code::<CODE_NAME> that is equals to 4 in the protobuf library. Usually using a digit directly can be confusing although I really appreciate the comment explaining what it is!

Hectorhammett · 2026-07-30T23:45:36Z

+                    self::fullyQualifiedBackupName(self::$backupId1)
+                );
+            } catch (ConflictException $e) {
+                $this->assertTrue(true); // Expected exception


This is a nit but if I recall correctly there is a phpunit function for asserting exceptions:

$this->assertException()

I like using the designated method but I can also see an argument of using the tools to simplify our lives and keep it like it is.

Hectorhammett · 2026-07-31T00:01:10Z

+        try {
+            foreach (self::$instance->backupOperations() as $op) {
+                if (!$op->done()) {
+                    $metadata = $op->info()['metadata'] ?? [];
+                    if (isset($metadata['database']) && $metadata['database'] === $dbFullName) {
+                        try {
+                            $op->cancel();
+                            $op->pollUntilComplete(['maxPollingDurationSeconds' => 120]);
+                        } catch (\Exception $e) {
+                            // Ignore exceptions from cancelled operations
+                        }
+                    }
+                }
+            }


try { foreach(self::$instance->backupOperations() as $op) { if ($op->done()) { continue; } $metadata = $op->info()['metadata'] ?? []; if (!isset($metadata['database']) || $metadata['database'] !== $dbFullName) { continue; } try { $op->cancel(); $op->pollUntilComplete(); } catch (\Exception $e) { //ignore errors } } } catch (\Exception $e) { //ignore errors }

trying to one up me on the meme game I see

product-auto-label Bot added the api: spanner Issues related to the Spanner API. label Jul 27, 2026

cy-yun changed the title ~~fix(Spanner): Make system tests work~~ fix(Spanner): Make System Tests Work & Work Faster Jul 27, 2026

cy-yun force-pushed the fix-spanner-system-tests branch 2 times, most recently from a0bfe04 to 6f74d5e Compare July 27, 2026 21:26

cy-yun marked this pull request as ready for review July 28, 2026 18:23

cy-yun requested a review from a team as a code owner July 28, 2026 18:23

cy-yun assigned bshaffer Jul 28, 2026

cy-yun force-pushed the fix-spanner-system-tests branch from b6fc922 to 03cee14 Compare July 28, 2026 21:26

bshaffer reviewed Jul 28, 2026

View reviewed changes

cy-yun force-pushed the fix-spanner-system-tests branch 10 times, most recently from bed320e to 8532688 Compare July 30, 2026 19:40

cy-yun requested a review from a team as a code owner July 30, 2026 19:40

cy-yun force-pushed the fix-spanner-system-tests branch from 8532688 to 8121741 Compare July 30, 2026 19:42

cy-yun added 7 commits July 30, 2026 15:15

chore: enable Spanner system tests in CI

0de77c7

fix(spanner): fix test database initialization and teardown on persis…

da47cc0

…tent databases, and apply code review fixes for tests

test(spanner): rollback open transaction in ReadTest

fd50515

test(spanner): clear stale backups before running BackupTest

60871d1

cy-yun force-pushed the fix-spanner-system-tests branch from f321d02 to 60871d1 Compare July 30, 2026 22:15

Hectorhammett requested changes Jul 31, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(Spanner): Make System Tests Work & Work Faster - #9401

fix(Spanner): Make System Tests Work & Work Faster#9401
cy-yun wants to merge 7 commits into
mainfrom
fix-spanner-system-tests

cy-yun commented Jul 27, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Hectorhammett Jul 30, 2026

Uh oh!

Hectorhammett Jul 30, 2026

Uh oh!

Hectorhammett Jul 30, 2026

Uh oh!

Hectorhammett Jul 31, 2026 •

edited

Loading

Uh oh!

cy-yun Jul 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

cy-yun commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Test Database Provisioning Overhead & DDL Consolidation

2. PostgreSQL DDL Compatibility & Schema Mismatches

3. Row ID Collisions on Persistent Databases

4. BackupTest Reliability & Timeouts

5. Emulator Transaction Leaks

6. CI Configuration

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Hectorhammett Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Hectorhammett Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Hectorhammett Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Hectorhammett Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cy-yun Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cy-yun commented Jul 27, 2026 •

edited

Loading

4. `BackupTest` Reliability & Timeouts

Hectorhammett Jul 31, 2026 •

edited

Loading